Prepare workflows for the zizmor bump in #360 - #365
Conversation
basecamp#360 bumps zizmor-action v0.5.3 -> v0.5.7, which ships a newer zizmor that reports five findings the pinned version does not. Two are pre-existing here and one is an artifact of how dependabot writes version comments. adhoc-packages (2, pre-existing): gem install bundler-audit and npm install -g npm@latest. Both are deliberate. bundler-audit is the auditing tool itself, and the npm upgrade is required for OIDC trusted publishing. Suppressed inline with justifications, matching the fifteen suppressions already in these workflows. ref-version-mismatch (3, latent): the trivy-action pin is commented # 0.35.0. That tag exists, so it is valid today, but upstream stopped publishing unprefixed tags after it -- ed142fd (0.36.0) is only reachable as v0.36.0. Dependabot copies the existing comment style, so it wrote # 0.36.0 in basecamp#360 and zizmor flagged an unknown ref. Switching to # v0.35.0 makes the next bump resolve correctly on its own. With these in place basecamp#360 should go green on a rebase, with no edits to its branch.
Sensitive Change Detection (shadow mode)This PR modifies control-plane files:
|
There was a problem hiding this comment.
Pull request overview
Prepares the repo’s GitHub Actions workflows to pass a newer zizmor version (as part of zizmor-action bump in #360) by aligning Trivy pin comments with upstream tag naming and suppressing two intentional “ad-hoc” package installs.
Changes:
- Updated
aquasecurity/trivy-actionpin comments from# 0.35.0→# v0.35.0to avoidref-version-mismatchfindings on future Dependabot bumps. - Added inline
zizmor: ignore[adhoc-packages]suppressions (with justification) forgem install bundler-auditandnpm install -g npm@latest.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| .github/workflows/security.yml | Adjusts Trivy pin comment format and suppresses intentional bundler-audit install to satisfy newer zizmor checks. |
| .github/workflows/release-typescript.yml | Suppresses intentional global npm upgrade step to satisfy newer zizmor checks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The publish job installed npm@latest from the registry, unpinned, inside the job that holds id-token: write and the release-npm environment, three steps before npm publish. A compromised install there could publish a malicious @37signals/basecamp rather than merely read a public repo. Node 24 bundles npm 11.16, past the 11.5.1 that OIDC trusted publishing requires, so the upgrade step is unnecessary. Bumping the publish job to Node 24 removes the ad-hoc install entirely: npm now comes from the Node tarball, which setup-node verifies by checksum. This removes the finding rather than suppressing it. Verified under Node 24 (npm 11.16.0): npm ci, npm run build (what prepublishOnly runs), and the full suite, 616 tests across 64 files.
Main bumped the shared action pins (#360/#365/#367 zizmor prep + dep bumps) after this workflow was authored: checkout v7.0.0, setup-go v6.5.0, setup-node v6.4.0, setup-java v5.4.0, setup-gradle v6.2.0, setup-ruby v1.316.0, setup-uv v8.2.0. Copy test.yml's exact pin lines so live-canary.yml doesn't reintroduce superseded SHAs. upload-artifact already matched.
Main bumped the shared action pins (#360/#365/#367 zizmor prep + dep bumps) after this workflow was authored: checkout v7.0.0, setup-go v6.5.0, setup-node v6.4.0, setup-java v5.4.0, setup-gradle v6.2.0, setup-ruby v1.316.0, setup-uv v8.2.0. Copy test.yml's exact pin lines so live-canary.yml doesn't reintroduce superseded SHAs. upload-artifact already matched.
Main bumped the shared action pins (#360/#365/#367 zizmor prep + dep bumps) after this workflow was authored: checkout v7.0.0, setup-go v6.5.0, setup-node v6.4.0, setup-java v5.4.0, setup-gradle v6.2.0, setup-ruby v1.316.0, setup-uv v8.2.0. Copy test.yml's exact pin lines so live-canary.yml doesn't reintroduce superseded SHAs. upload-artifact already matched.
Main bumped the shared action pins (#360/#365/#367 zizmor prep + dep bumps) after this workflow was authored: checkout v7.0.0, setup-go v6.5.0, setup-node v6.4.0, setup-java v5.4.0, setup-gradle v6.2.0, setup-ruby v1.316.0, setup-uv v8.2.0. Copy test.yml's exact pin lines so live-canary.yml doesn't reintroduce superseded SHAs. upload-artifact already matched.
…eduled CI (#308) * conformance: pairwise BC4↔BC5 schema + canonical memories rule Extends `conformance/schema.json` with `pairwiseAssertions[]` and four rule types: pairwiseSupersetArray, pairwiseSupersetKeys, pairwiseEqual, pairwiseDeltaAllowed. The first three enforce the additive-only invariant between BC4 and BC5 snapshots of the same operation; the fourth is a per-test allowlist with a required `reason` so a future public compatibility report can index accepted divergences. Per-backend schema validation alone can't catch this class of regression: with every new BC5 field marked optional, a hypothetical case where BC4 emits `memories: [items]` and BC5 emits `memories: []` passes each backend's schema independently. The pairwise rules close that loop. Path syntax: dotted identifiers relative to each snapshot. The empty string addresses the body root; `foo.bar` defaults to `pages[0].body.foo.bar` for single-page snapshots; explicit `pages[N].body.X` or aggregate `pages[*].body.X` are available when needed. Adds the canonical canary rule on `GetMyNotifications`: BC3 commit 64acf34 aliases BC5 memories[] to bubble_ups[] so BC4 API consumers keep receiving the same population on BC5; the pairwiseSupersetArray rule fires immediately if that alias ever drops. A pairwiseSupersetKeys rule on the body root catches any other key removal between backends. * scripts: pairwise canary comparison Implements `scripts/compare-canary-runs.sh`, the executable side of the pairwise rules added in the prior commit. Reads BC4 and BC5 snapshot directories (one wire snapshot per operation, format established by the TS live-runner), looks up each test's `pairwiseAssertions` in `live-my-surface.json`, and applies the four rule types in turn. Path resolution: `""` → body root, `foo.bar` → `pages[0].body.foo.bar`, `pages[N].body.X` → explicit page, `pages[*].body.X` → aggregate across pages (collected into a list, so SupersetArray on the aggregate checks "number of pages emitting this", not a sum of inner lengths; that's an intentional semantic — multi-page aggregation is the escape hatch, single-page page-relative addressing is the common case). Exit codes: 0 clean, 1 violations, 2 operator error. Violation output groups by operation and includes the rule + display path so the specific invariant that fired is obvious without re-running. Identical account state across the two runs is mandatory for pairwise results to be meaningful; CONTRIBUTING.md documents this in the forthcoming docs commit. * make: conformance-live + check-bc5-compat orchestrators Adds two opt-in targets that thread the live canary pipeline end-to-end. `make conformance-live` runs one backend's pass: TS captures canonical wire snapshots, then Ruby/Python/Go/Kotlin replay-decode them. Requires LIVE_RECORD_DIR + BASECAMP_BACKEND in the env; not invoked by `make check`. `make check-bc5-compat` is the top-level orchestrator: it wipes LIVE_RECORD_DIR (default `tmp/live-canary`), runs `conformance-live` once with BASECAMP_BACKEND=bc4, again with BASECAMP_BACKEND=bc5 against BC5_HOST, then invokes `scripts/compare-canary-runs.sh` on the two captured snapshot directories. Failure at any stage fails the whole target. The TS runner writes to `$LIVE_RECORD_DIR/$BASECAMP_BACKEND/wire/`, so one LIVE_RECORD_DIR root is reused across both passes and the per- backend distinction lives in BASECAMP_BACKEND. The plan documents this explicitly to avoid a separate-dir mental model that would diverge from what the TS runner actually does. * ci+docs: live-canary scheduled workflow + CONTRIBUTING pairwise section Adds `.github/workflows/live-canary.yml` — a nightly cron + manual workflow_dispatch trigger that runs `make check-bc5-compat`. Gated on repo secrets (BASECAMP_TOKEN, BASECAMP_ACCOUNT_ID, BC5_HOST); the job no-ops with a clear log message when the secrets aren't configured rather than failing the run, so forks and clones without canary access don't break. Sets up the full polyglot toolchain mirroring the existing Conformance Tests job — Go, Node, Ruby, Python, Java/Gradle — since the orchestrator invokes one capture + four replays. Uploads the entire `tmp/live-canary` tree as a workflow artifact with 14-day retention so failures can be inspected post-hoc without rerunning a costly live capture. CONTRIBUTING.md "Live canary" section gains a "Pairwise BC4↔BC5 comparison" subsection (the four rule types, the path syntax, the canonical `memories` example), an "Orchestrator" subsection (the make target wiring with the BC4-then-BC5-then-compare flow), and a "Scheduled CI" subsection (secret requirements + opt-in behavior). The stale "PR 4 lands the orchestrator" reference removed; the live infra is now the documented state, not a forward promise. Identical account state across the two runs is the load-bearing operational requirement and is called out explicitly in the docs — the pairwise rules assume the BC4 and BC5 backends see equivalent fixtures; without that, `unreads`-style natural drift will false-fail rules. * docs: drop stale PR-numbered references to canary infra PR 4 is now shipping the orchestrator + pairwise comparison, so the forward-looking "PR 3 / PR 4" markers in CONTRIBUTING and COORDINATION no longer describe pending work — they describe what's in this commit's parent series. Rewrite the references to name the artifacts directly (`make conformance-*-replay`, `scripts/compare-canary-runs.sh`, `make check-bc5-compat`) so the docs stay accurate after PR sequencing shifts. * conformance: address PR #308 review threads + reframe memories canary rule Post-launch reconciliation for the pairwise BC4<->BC5 canary. Clears the 14 review threads on #308 and corrects the memories rule to match what BC5 master actually shipped. scripts/compare-canary-runs.sh: - P0 (codex/cubic): match the TS live runner's snapshot filename scheme exactly -- sanitize the test name with [^A-Za-z0-9_-]+ -> "_" (case preserved), not lowercase/hyphen/operation-first. The old form matched no files, so check-bc5-compat could report clean without comparing anything. - P1 (cubic): a declared pairwise test missing a snapshot on either backend is now a hard error (exit 2), not a silent skip-and-pass. - empty `paths` loop (copilot): guard ${#RULE_PATHS[@]} instead of "${RULE_PATHS[@]:-}" so an empty array can't run against the body root. - pairwiseEqual (cubic): compare via jq deep-equality so object key-order differences don't false-fail. - exit-code contract (copilot): snapshot-read jq failures remap to the documented exit 2 instead of leaking jq's status under set -e. conformance/schema.json: pairwiseAssertions[].paths gains minItems:1. Makefile: - rm -rf "$(LIVE_RECORD_DIR)" guarded against empty / "/" / "..". - conformance-live validates LIVE_RECORD_DIR/BASECAMP_BACKEND before the TS live pass (invoked from the recipe after the guards) rather than after it. - check-bc5-compat comment: "reports all violations", not "fails on first". - new check-compare-canary target, wired into `make check`. .github/workflows/live-canary.yml: default BASECAMP_HOST in the workflow expression so an unset optional secret can't clobber the Makefile default. CONTRIBUTING.md: align the "all violations" wording; reframe the memories example to the waiver pattern. memories canary rule (conformance/tests/live-my-surface.json): the prior "BC3 commit 64acf34 aliases memories[] to bubble_ups[]" claim is counterfactual -- that commit does not exist, and BC5 master shipped `json.memories []` while BC4 four still populates it. The pairwiseSupersetArray rule now states the intended invariant and is waived by a temporary pairwiseDeltaAllowed entry pointing at spec/api-gaps/memories-emptied-regression.md (added on the reconciliation branch); remove the waiver once BC3 #10947 (which carries `json.memories @bubble_ups`) merges. scripts/test-compare-canary-runs.sh: network-free regression coverage for the filename scheme (incl. proof the old form finds nothing), missing-snapshot hard-fail, memories-waiver scoping, pairwiseEqual key-order, and the empty-paths guard. * conformance: clarify BC4 branch reference in memories waiver reason The waiver `reason` strings read "BC4 four still populates it", which scans as a typo to anyone without the post-launch topology in hand (BC4 is the `four` branch). Reword to "BC4 (the four branch)" in the live-my-surface.json description + pairwiseDeltaAllowed reason and the CONTRIBUTING.md example. Addresses the Copilot re-review on PR #308. * ci(live-canary): align action pins with main's bumped SHAs Main bumped the shared action pins (#360/#365/#367 zizmor prep + dep bumps) after this workflow was authored: checkout v7.0.0, setup-go v6.5.0, setup-node v6.4.0, setup-java v5.4.0, setup-gradle v6.2.0, setup-ruby v1.316.0, setup-uv v8.2.0. Copy test.yml's exact pin lines so live-canary.yml doesn't reintroduce superseded SHAs. upload-artifact already matched. * conformance: strict pairwiseSupersetKeys types + decouple harness from full fixture Two Copilot findings on the rebased branch: * pairwiseSupersetKeys only flagged the BC4-object/BC5-non-object case; any other non-null non-object (array/string/number) on either side silently did nothing, hiding a mis-specified path or a real object → scalar shape change. Null still counts as the empty key set (absent-on-one-side stays additive-friendly and a dropped object now reports its missing keys); anything else is a violation, mirroring pairwiseSupersetArray's INVALID handling. New scenarios G (scalar at a keys path fails, no silent skip) and H (BC4 keys vs BC5 null reports the missing keys) pin both semantics. * The regression harness ran scenarios A-D against the entire real live-my-surface.json, so adding any future pairwise-bearing test to the real fixture would break them with missing-snapshot hard errors for tests the harness never writes snapshots for. A-D now run against a filtered tests file holding just the verbatim-real GetMyNotifications entry (startup asserts exactly one match). * conformance: fail loud when the pairwise tests file can't be parsed mapfile -t TEST_ENTRIES < <(jq ...) hid jq failures — a process substitution's exit status is invisible to set -e, so a malformed tests file left TEST_ENTRIES empty and the script exited 0 as 'nothing to compare', silently masking an operator error and breaking the 0/1/2 exit-code contract. Materialize the jq output first and exit 2 with a parse error when jq fails. Scenario I pins the behavior. * conformance: reject non-array tests files, fast env preflight, 3.81-safe defaults Three review threads plus a latent bug the preflight work surfaced: * The tests-file parser's jq program started with '.[]', which iterates object values too — a top-level object was misinterpreted instead of treated as an operator error. An explicit type guard now errors inside jq (caught by the existing exit-2 path). Scenario J pins it. * conformance-live preflights BASECAMP_TOKEN and BASECAMP_ACCOUNT_ID in the Makefile so a missing credential fails in milliseconds with a clear source instead of deep inside the vitest run after npm ci. The doc comment no longer lists BASECAMP_LIVE=1 as caller-required — the conformance-typescript-live recipe sets it itself. * check-bc5-compat's target-specific '?=' defaults are gone: GNU make 3.81 (macOS /usr/bin/make) leaks 'target: VAR ?= default' into a global override that clobbers the caller's environment to empty for every OTHER target's recipe — with the makefile merely loaded, 'conformance-live' rejected a perfectly good LIVE_RECORD_DIR from the environment. Defaults now resolve in the recipe shell (${LIVE_RECORD_DIR:-tmp/live-canary}), which behaves identically on make 3.81 and 4.x. Verified under 3.81: conformance-live now sees caller env; the rm -rf safety guard still refuses '/' and '..'. * ci(live-canary): update BASECAMP_HOST default rationale comment The comment referenced the Makefile's 'BASECAMP_HOST ?=' default, which the previous commit replaced with recipe-shell ${VAR:-default} resolution (which also catches the empty-string expansion of an unset secret). The workflow-level default stays for self-description; the comment now says why. * conformance: pages[*] superset arrays compare item totals; bash>=4 guard Two review threads: * pairwiseSupersetArray on an aggregated pages[*] path compared the outer array length — which is just the page count — so a field with arrays on every BC4 page and dropped on every BC5 page false-greened whenever page counts matched (codex P2). Aggregated paths now compare the TOTAL item count across pages: an absent (null) page value contributes 0, any non-null non-array page value poisons the total as INVALID. Page redistribution with the total preserved stays green — page boundaries aren't contract. Scenarios K (dropped-on-every-page fails on totals) and L (redistribution passes) pin both sides. * Explicit bash >= 4 guard at the top: the script uses mapfile, and macOS's /bin/bash 3.2 failed with an opaque 'mapfile: command not found' mid-script instead of a clear operator error. * conformance: materialize remaining jq reads; keep make -n non-destructive Round follow-ups: * ALLOW_PATHS, ENFORCED_RULES, and RULE_PATHS got the same materialize-before-mapfile treatment as TEST_ENTRIES — a jq failure inside a process substitution is invisible to set -e, so extraction errors could silently yield empty arrays (skipping the allowlist or all rules). Emptiness now drives off jq's own length so the empty-string body-root sentinel path ("") is never confused with an empty list, with count-mismatch checks failing loud (exit 2). * check-bc5-compat's rm -rf moved back to its own recipe line: under 'make -n', lines containing $(MAKE) still execute (with -n propagated), so the consolidated block from the 3.81 fix deleted snapshots during dry-runs. Guards + rm now sit on a $(MAKE)-free line that -n only prints; shell-level defaults are recomputed per line. Verified: a marker file under tmp/live-canary survives 'make -n check-bc5-compat'; the 3.81 env-clobber fix and the rm safety guards are unchanged. * conformance: refuse rm -rf on '.'; explicit error for non-array paths * The rm -rf guard now strips a trailing slash and refuses '', '.', and '/' — LIVE_RECORD_DIR=. (or ./) would have deleted the repo checkout, a realistic operator slip for a locally-run target. Verified both forms are refused with exit 2. * A non-array 'paths' on a rule (string or object) now exits 2 with "'paths' ... must be an array of strings" instead of leaking jq's raw exit status (string case) or silently iterating object values as paths (object case, which jq accepts without error). The count expression type-guards before the element extraction; scenario M pins the contract. * conformance: preserve trailing body-root paths, validate snapshot shape, full slash-strip Round follow-ups, all three verified with new harness scenarios: * Path extraction switched to NUL-delimited jq output (mapfile -d ''). Newline splitting through command substitution strips trailing blank lines, so a valid rule ending with the documented body-root sentinel (paths: ["foo", ""]) lost the final path and exit-2'd on the count mismatch. NUL delimiters preserve empty-string paths; the count check remains as the backstop for jq failures inside the process substitution and for non-string elements. Requires bash >= 4.4 for mapfile -d (guard bumped from 4.0 with the same clear error). Scenarios O (trailing "" evaluated, exit 0) and O2 (its violation still fails, exit 1) pin both sides. * Wire snapshots are validated structurally after the existence check: valid JSON alone ('{}', '{"pages":{}}') made every read return null, silently false-greening superset rules. The TS runner's contract is enforced: object, non-empty pages array, pages_count matches. Scenario N pins it. * The rm -rf guard strips ALL trailing slashes before the case check — a single strip let LIVE_RECORD_DIR=/// through as '//', which rm resolves to root. Verified /// now refuses with exit 2; the error reports the caller's original value. * conformance: absolutize LIVE_RECORD_DIR, validate waiver types + page shape Four review threads: * conformance-live canonicalizes LIVE_RECORD_DIR to an absolute path before fanning out (codex P1): the capture and replay recipes each cd into their runner directory, so the documented relative default tmp/live-canary scattered snapshots under conformance/runner/*/ while the pairwise compare read from the repo root — a configured check-bc5-compat run failed with missing snapshots instead of comparing. Children now receive the absolutized path explicitly. * A pairwiseDeltaAllowed waiver whose 'paths' is a bare string was flattened into a single valid allowed path, silently suppressing the enforcing rule. The extraction now requires each waiver's paths to be an array of strings and exits 2 otherwise. Scenario P pins it (bare-string waiver + real regression → exit 2, not a pass). * validate_snapshot enforces the documented per-page shape ({status, headers, body, bodyText, url}) — pages like [{}] passed the count check but read as null, false-greening rules against a damaged baseline. Scenario Q pins it. * The regression harness fails fast with the same bash >= 4.4 guard as the compare script, instead of sixteen confusing scenario failures under macOS system bash. * docs: align pages[*] path semantics with the compare implementation The bullet suggested pages[*] was for how-many-pages checks, but pairwiseSupersetArray compares the total item count across pages (absent page values contribute 0, non-array page values are invalid). Say so, so operators don't write rules against the old semantics. * conformance: waivers require a non-empty reason at runtime schema.json requires a reason on pairwiseDeltaAllowed, but the compare script also runs standalone (check-bc5-compat, the scheduled workflow) with no schema-validation step in front of it — a reason-less waiver still flattened into ALLOW_PATHS and suppressed enforcement unaudited. The extraction now rejects waivers without a non-empty string reason (exit 2). Scenario R pins it; scenario P's assertion follows the combined waiver error message. * conformance: rm -rf -- for option-injection robustness A LIVE_RECORD_DIR (or, theoretically, a mktemp result) beginning with '-' would be parsed as an rm option; end option parsing explicitly. * docs(make): note the bash >= 4.4 requirement on check-compare-canary The compare + harness scripts guard for it with a clear error; the Makefile comment now says so up front for contributors running make check on stock macOS bash. * conformance: snapshot/test operation integrity; schema forbids empty waiver reason * Each snapshot's recorded .operation must match the test entry's operation before comparison — a stale or overwritten file with the right name exits 2 instead of being compared as this test's capture. Scenario S pins it. * conformance/schema.json aligns with the runtime waiver guard: the pairwiseDeltaAllowed conditional now sets minLength 1 on reason, so an empty-string reason fails schema validation the same way it fails the compare script. TS conformance suite re-run green (84 passed). * conformance: waiver paths must be non-empty at runtime schema.json's minItems:1 on paths doesn't run in front of standalone invocations (check-bc5-compat, the scheduled workflow). An empty waiver paths array is functionally harmless (it waives nothing) but is always a fixture mistake — reject it with the same exit-2 operator error as the other waiver shape violations. Scenario T pins it. * conformance: segment-aware .. guard, jq preflight, broader read-error message * The rm -rf guard now refuses '..' only as a PATH SEGMENT (leading, interior, trailing, or the whole path) instead of any substring, so benign names like tmp/live-canary..pr308 are allowed while every traversal form still refuses. Patterns unit-verified for all six shapes. * The regression harness preflights jq with the same clear error as the compare script instead of failing mid-setup with a raw command-not-found. * snapshot_read_error's hint no longer blames only malformed JSON — an invalid jq path expression built from a rule's path entry lands in the same handler. * docs: pairwiseSupersetKeys does not take pages[*] aggregates The aggregate is a list, not an object — the rule flags it as an invalid target rather than evaluating it. Say so and point rule authors at pages[N].body.X for keys checks on paginated endpoints. * conformance: '[*]' only as the leading pages[*] segment The aggregate handling keyed off any '[*]' substring, broader than the documented pages[*].body.X form — an undocumented items[*].foo would stream through jq with undefined comparison semantics instead of surfacing the fixture mistake. Both the read and the supersetArray totals branch now see only validated leading-pages[*] paths; anything else exits 2 as an unsupported path. Scenario U pins it. * docs: bash/jq prerequisites; pin double-star rejection * CONTRIBUTING's prerequisites now list the shared tooling make check needs beyond per-SDK toolchains: jq, and bash >= 4.4 for the pairwise-canary scripts (macOS /bin/bash is 3.2 — brew install bash; the scripts fail fast with that exact hint). * Scenario U2 pins that a second '[*]' after a valid leading pages[*] is rejected as unsupported (the guard already covered it). * conformance: refuse checkout-ancestor LIVE_RECORD_DIR; mktemp template * An absolute LIVE_RECORD_DIR equal to the repo checkout (or any of its ancestors, e.g. LIVE_RECORD_DIR=$PWD or $HOME) passed the guards and rm -rf'd the working tree before the canary ran. A new prefix check refuses any path the checkout lives under; absolute paths elsewhere (/tmp/canary) and in-checkout subdirs still work. Patterns unit-verified for checkout, $HOME, /tmp, relative, and in-checkout-absolute shapes. * The harness's mktemp gets an explicit template — current macOS accepts a bare -d (empirically verified here), but older BSD variants insist on a template and it costs nothing. * conformance: reframe memories waiver as permanent documented contract divergence bc3's BC5 API train shipped 2026-07-18..21 and settled the memories question: doc/api/sections/my_notifications.md (bc3 #11628) documents memories as an always-empty placeholder superseded by bubble_ups, and bc3 #10947 — the once-planned `json.memories @bubble_ups` alias — is closed unmerged. The pairwiseDeltaAllowed entry is therefore not a temporary waiver pending a fix; it is the permanent machine-readable record of an accepted BC4→BC5 subtractive delta. Update the GetMyNotifications test description and both rule reasons in live-my-surface.json, and align the canonical example + prose in CONTRIBUTING.md, replacing the 'drop the waiver once #10947 merges' guidance with the actual retire conditions: BC4 empties memories too, or BC5's documented contract changes. * conformance: validate rule paths before applying the waiver skip is_allowed's early continue ran before the '[*]' syntax guard, so an unsupported path (items[*].foo, a double star) that also appeared in a pairwiseDeltaAllowed waiver exited clean instead of reporting the fixture error — a typo in a waived canary rule could permanently suppress enforcement without ever surfacing. Validate path syntax first, then apply the allowlist skip. Adds scenario U3 covering the waived-unsupported-path case. Flagged by Codex review. * conformance: reject unknown rule types before applying path waivers The unknown-RULE_TYPE rejection lived in the dispatch case inside the per-path loop, which only runs for paths that survive the is_allowed skip. A misspelled type (pairwiseSupersetAray) whose paths were all waived by pairwiseDeltaAllowed therefore exited clean — an operator error silently suppressed, and the typo'd rule never enforced anything again. Hoist the type check to rule extraction, before any per-path processing; the case's catch-all arm stays as defense in depth. Adds scenario V covering the waived-misspelled-type case. Flagged by Codex review. * conformance: reject all undocumented bracket paths, not just '[*]' The star guard only matched the literal '[*]', so a bare jq-stream typo like items[] (or pages[0].body.items[]) reached read_value unwrapped: the stream made the length variables multi-line/empty, the -lt comparison errored falsy, and a real shrink violation (BC4 [1,2] vs BC5 []) exited 0 — a false-green on exactly the invariant the script enforces. Generalize the guard: brackets are only accepted as a leading pages[N] or pages[*] segment (regex-validated), everything else is rejected as a fixture mistake with exit 2, still ahead of the waiver skip. Adds scenario W reproducing the items[] false-green. Flagged by Codex review. * conformance: harden pairwise path + snapshot validation (review round 4) - extract validate_rule_path() and apply it to pairwiseDeltaAllowed entries too: a malformed bracket path appearing only in a waiver names a path no enforcing rule can ever target and previously sat unnoticed - require bracketed paths to enter the page body (pages[N].body...): page-level keys other than body are backend-specific metadata with no pairwise semantics, and a typo like pages[0].items read null on both snapshots and false-greened superset rules - validate_snapshot now type-checks page keys (status number, headers object, bodyText string, url string) so typed corruption fails loud instead of reading as nulls - check-bc5-compat rm guard: canonicalize an existing LIVE_RECORD_DIR (cd + pwd -P) before the checkout-ancestor check so non-canonical spellings can't slip past the string compare; a failed cd empties LRD, which the ancestor pattern then refuses Regression scenarios X (waiver-only malformed path), Y (bracket path skipping .body), Z (wrongly-typed page keys) added; 29/29 green. * conformance: reject pages[N] rules whose page is absent from a capture A concrete pages[N] rule asserts the page exists on both backends. When either capture had fewer pages, read_value returned null and the rule passed clean — hiding a fixture typo (pages[1] against single-page captures) or a genuine page-count regression. Both demand operator attention: fail with exit 2 naming the short snapshot and its page count. Cross-page-count comparison remains the job of pages[*] aggregation. Regression scenarios AA (absent from both) and AB (one-sided page loss, error names the bc5 snapshot) added; 31/31 green. * conformance: whitelist rule-path grammar instead of blacklisting brackets A path with jq punctuation but no brackets ('items, .pages') slipped the bracket blacklist and interpolated into the jq program as raw syntax. Terminate the class: validate_rule_path now admits only the documented grammar — dotted [A-Za-z0-9_-] key segments, optionally led by a pages[N]/pages[*] indexer that must enter the page body, plus the "" body-root sentinel — and rejects everything else as a fixture mistake. Regression scenario AC (comma path) added; 32/32 green. * conformance: drop hyphens from path grammar; stop uploading raw wire snapshots - validate_rule_path: remove '-' from the segment charset — '.foo-bar' parses in jq as subtraction ('.foo - bar'), not a key lookup, so a hyphen-keyed rule evaluated to garbage instead of reading the field. No fixture path uses hyphens; keys requiring quoting are a future, deliberate extension of to_jq_path. Regression scenario AD added (33/33 green). - live-canary.yml: upload only the decode/ result trees. Wire snapshots carry raw response bodies and captured headers from a live account and must not persist as downloadable workflow artifacts; decode results carry only booleans, error strings, and dotted field paths. * make: rm the original LIVE_RECORD_DIR path, guard on the canonical one The round-7 canonicalization rewrote LRD itself, so rm -rf on a symlinked LIVE_RECORD_DIR deleted the tree the link points to instead of the link — a behavior change with data-loss potential. Canonicalize into a separate guard variable for the checkout-ancestor check and keep the rm on the original (trailing-slash-stripped) path: a symlink is removed as a link, while a symlink pointing into the checkout is still refused by the guard (verified both by hand). * conformance: redact live values from pairwiseEqual violation output A pairwiseEqual failure printed the complete BC4 and BC5 JSON values into the comparison output, which the scheduled canary lands in retained, log-access-readable Actions logs — leaking live-account field content (names, URLs, notification text) on any drift. Report only shape summaries (type + keys/length) and point the operator at a local re-run for raw values. Regression scenario AE proves the violation still fires while the content stays out of the output; 34/34 green. * conformance: redact live values from superset invalid-target violations Completes 5270e94: the invalid-target branches of pairwiseSupersetArray and pairwiseSupersetKeys still embedded the full BC4/BC5 JSON values in violation output. Factor the shape summary into a shared shape_of() helper and use it at all three sites; no violation message carries live field content anymore (grep-verified zero BC4=$BC4_VAL sites). * conformance: require explicit page syntax once a capture paginates Shorthand paths ("" or "foo.bar") normalize to pages[0].body — fine for single-page captures, but against a multi-page snapshot they silently checked only page 0 and ignored the rest, under-enforcing the rule. Hard-error with a pointer to pages[N]/pages[*] syntax as soon as either side's capture has more than one page. Regression scenario AF added; 35/35 green. * conformance: use explicit ERE in scenario X's grep The \| alternation in a plain grep pattern is a GNU basic-regex extension; BSD grep treats it literally. grep -E with an unescaped pipe is unambiguous everywhere. * make: refuse a LIVE_RECORD_DIR that canonicalizes to filesystem root A symlink to / (or /.) passed the checkout-ancestor guard by string accident: the canonical root makes the pattern '//' + '*', which never matches an absolute pwd. No destructive path actually remained — rm operates on the original path, so a root symlink lost only the link and rm refuses trailing-dot operands — but the guard's invariant should be explicit, not an accident of rm semantics. Refuse a canonical '/' outright (verified by hand with a symlink to /).
#360 bumps
zizmor-actionv0.5.3 → v0.5.7, which ships a newer zizmor (1.26.x). That version reports five findings the currently pinned zizmor does not, so #360 failsGitHub Actions auditfor reasons unrelated to the twenty actions it bumps. zizmor exits non-zero on any finding, so all five must be resolved.Four of the five are not #360's doing. This clears them on
mainfirst, so #360 can go green on a plain rebase without anyone hand-editing a dependabot branch. Of the four, three are fixed, one is removed at the source, and exactly one is suppressed.adhoc-packagesin the publish job — removed, not suppressedrelease-typescript.ymlrannpm install -g npm@latest: an unpinned install from the registry, inside thepublishjob that holdsid-token: writeand therelease-npmenvironment, three steps beforenpm publishof@37signals/basecamp.The blast radius is not "a linter is unhappy." A compromised
npm@latestthere executes against a live OIDC publishing identity and can push a malicious release of this SDK. Pinning the version would narrow that, but zizmor still flags it — the rule is about installing outside a lockfile at all — so a pin buys a suppression, not a fix.Node 24 bundles npm 11.16, past the 11.5.1 that OIDC trusted publishing requires. The step is simply unnecessary. The publish job now uses Node 24 and takes npm from the Node tarball, which
setup-nodeverifies by checksum. The ad-hoc install is gone rather than accepted.Verified locally under Node 24 (npm 11.16.0):
npm ci,npm run build(whatprepublishOnlyruns, includingpostbuild), and the full suite — 616 tests across 64 files, all passing.adhoc-packagesin the bundler-audit job — suppressed, with reasongem install bundler-auditremains. That job hascontents: read, noid-token, no secrets, and does not publish; a compromised install reads a public repository. bundler-audit is also the auditing tool itself, so adding it to the SDK'sGemfilewould make the gem under audit depend on its own auditor.This is the one accepted risk, suppressed inline with a justification, matching the fifteen
# zizmor: ignore[...]suppressions already in these workflows.ref-version-mismatch— 3 medium, latentThe
trivy-actionpin is commented# 0.35.0. That tag exists, so it is valid today andmainis not currently flagged.Upstream stopped publishing unprefixed tags after it:
ed142fdis reachable only asv0.36.0, with no0.36.0. Dependabot mirrors the existing comment style, so when it bumped trivy in #360 it wrote# 0.36.0— an unknown ref — and the newer zizmor flagged it three times.Correcting the comment to
# v0.35.0means the next bump inherits thevprefix and resolves on its own. This improves auditability rather than reducing it: the comment exists so a reviewer can check the hash against a real tag.Verification
Three of the five findings come from zizmor's online audits, which silently do not run without a GitHub token — a plain local run reports clean and looks like a false alarm. All runs below had
GH_TOKENset.maintodayThe 2/3 split on #360's head matches its CI run exactly. The pinned zizmor 1.25.2 that
mainruns today also still passes here, so the unknown audit name in the ignore comment does not break current CI.The remaining fifth finding is #360's own trivy comment, which its rebase will regenerate as
# v0.36.0once this lands.